Add kernelCTF CVE-2024-53141_lts #150
Conversation
st424204
force-pushed
the
CVE-2024-53141_lts
branch
2 times, most recently
from
9e224ca to
80e6eee
Compare
st424204
force-pushed
the
CVE-2024-53141_lts
branch
from
80e6eee to
87044ae
Compare
koczkatamas
force-pushed
the
master
branch
3 times, most recently
from
933028d to
c111d81
Compare
koczkatamas
force-pushed
the
master
branch
5 times, most recently
from
fe221aa to
aa7067b
Compare
koczkatamas
force-pushed
the
master
branch
8 times, most recently
from
76d6b57 to
bf0d68f
Compare
There was a problem hiding this comment.
Hey,
This is just a quick code quality review. We're planning to review the submissions more deeply (actually understanding what the exploit does) in two weeks.
In general, the code quality looks acceptable, but we may have further comments during the deeper review. I've also left a few comments now.
We also have a draft style guide now. Please take a look and let us know if it's helpful for understanding our code quality expectations: https://google.github.io/security-research/kernelctf/style_guide.
Thanks for the submission and PR!
| } | ||
|
|
||
| // Function to create an IP set under kmalloc-cg-1024 cache | ||
| int create_ip_set_kmalloc_1024(int sock_fd, const char *set_name, const char *type_name, uint8_t family, int extra) |
There was a problem hiding this comment.
There seems to be a significant code reuse (copy-paste) in the following functions: create_ip_set_kmalloc_1024, trigger_oob_leak, create_ip_set_kmalloc_2048, add_ip_to_set and trigger_oob_write.
If you would create a helper function for the generic part and keep only the code which differs in these functions, the reader could more easily understand the differences.
See more details here: https://google.github.io/security-research/kernelctf/style_guide#code-duplication
| { | ||
| // if reach target loop, we spray a lot kmalloc-cg-2048 msg_msgseg ahead | ||
| for (int i = 0; i < 0x1000; i++) | ||
| SYSCHK(msgsnd(msqid[i], &msg, 0x1000 - 0x30 + 0x800 - 0x8, 0)); |
There was a problem hiding this comment.
Using PAGE_SIZE - MSG_MSG_SIZE + KMALLOC_CG_2K - MSG_MSGSEG_SIZE instead of 0x1000 - 0x30 + 0x800 - 0x8 can make it easier for readers to understand where these values come from, but in this case the comment about kmalloc-cg-2048 helps a lot as well.
More detail here: https://google.github.io/security-research/kernelctf/style_guide#approach-2
| } | ||
|
|
||
| // make comment_string be allocated under kmalloc-192 | ||
| memset(comment_string, 'a', 0x90); |
There was a problem hiding this comment.
This can be also moved into the trigger_oob_leak function directly or here directly before the trigger_oob_leak call as it's only used there (and comment_string can be a local variable instead of a global one). This way it would be easier to follow what's going on.
| SYSCHK(setrlimit(RLIMIT_NOFILE, &rlim)); | ||
|
|
||
|
|
||
| // prepare a lot unix_socket for spray skbs |
There was a problem hiding this comment.
Can you clarify how much allocation happens here? 0x400000 is 4MB, we allocate that 0x400 times for 2 FDs recv and sndbuf, that's 4M * 1024 * 4 = 16Gb of memory? That seems to be more than the VM has...
No description provided.